Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Remove unused locals in System.Data.Common and keep some logic using discard #41962

Merged
merged 2 commits into from
Oct 23, 2019
Merged

Remove unused locals in System.Data.Common and keep some logic using discard #41962

merged 2 commits into from
Oct 23, 2019

Conversation

eriawan
Copy link
Member

@eriawan eriawan commented Oct 21, 2019

remove unused locals in System.Data.Common and keep some logic using discard.
Partially fix issue #39962

cc @stephentoub

@eriawan
Copy link
Member Author

eriawan commented Oct 22, 2019

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 41962 in repo dotnet/corefx

@eriawan
Copy link
Member Author

eriawan commented Oct 22, 2019

Hmmm, apologize,
I thought I could re-run the Az DevOps pipelimes using that comment phrase. Ok, I hope the CI will be green again.

@stephentoub
Copy link
Member

I hope the CI will be green again.

It's broken because of this change, e.g.

There should be exactly 1 table here
   at System.Data.DataTable.DeserializeDataTable(SerializationInfo info, StreamingContext context, Boolean isSingleTable, SerializationFormat remotingFormat) in /_/src/System.Data.Common/src/System/Data/DataTable.cs:line 307
   at System.Data.DataTable..ctor(SerializationInfo info, StreamingContext context) in /_/src/System.Data.Common/src/System/Data/DataTable.cs:line 210
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
   at System.Reflection.RuntimeConstructorInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture) in /_/src/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs:line 297
   at System.Reflection.MethodBase.Invoke(Object obj, Object[] parameters) in /_/src/System.Private.CoreLib/shared/System/Reflection/MethodBase.cs:line 50
   at System.Runtime.Serialization.ObjectManager.CompleteISerializableObject(Object obj, SerializationInfo info, StreamingContext context) in /_/src/System.Runtime.Serialization.Formatters/src/System/Runtime/Serialization/ObjectManager.cs:line 771
   at System.Runtime.Serialization.ObjectManager.FixupSpecialObject(ObjectHolder holder) in /_/src/System.Runtime.Serialization.Formatters/src/System/Runtime/Serialization/ObjectManager.cs:line 192
   at System.Runtime.Serialization.ObjectManager.DoFixups() in /_/src/System.Runtime.Serialization.Formatters/src/System/Runtime/Serialization/ObjectManager.cs:line 820
   at System.Runtime.Serialization.Formatters.Binary.ObjectReader.Deserialize(BinaryParser serParser, Boolean fCheck) in /_/src/System.Runtime.Serialization.Formatters/src/System/Runtime/Serialization/Formatters/Binary/BinaryObjectReader.cs:line 104
   at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.Deserialize(Stream serializationStream, Boolean check) in /_/src/System.Runtime.Serialization.Formatters/src/System/Runtime/Serialization/Formatters/Binary/BinaryFormatter.cs:line 69
   at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.Deserialize(Stream serializationStream) in /_/src/System.Runtime.Serialization.Formatters/src/System/Runtime/Serialization/Formatters/Binary/BinaryFormatter.cs:line 41
   at System.Runtime.Serialization.Formatters.Tests.BinaryFormatterHelpers.FromByteArray(Byte[] raw, FormatterAssemblyStyle assemblyStyle) in /_/src/Common/tests/System/Runtime/Serialization/Formatters/BinaryFormatterHelpers.cs:line 123
   at System.Runtime.Serialization.Formatters.Tests.BinaryFormatterHelpers.FromBase64String(String base64Str, FormatterAssemblyStyle assemblyStyle) in /_/src/Common/tests/System/Runtime/Serialization/Formatters/BinaryFormatterHelpers.cs:line 131
   at System.Runtime.Serialization.Formatters.Tests.BinaryFormatterTests.ValidateAndRoundtrip(Object obj, TypeSerializableValue[] blobs, Boolean isEqualityComparer) in /_/src/System.Runtime.Serialization.Formatters/tests/BinaryFormatterTests.cs:line 120
   at System.Runtime.Serialization.Formatters.Tests.BinaryFormatterTests.ValidateAgainstBlobs(Object obj, TypeSerializableValue[] blobs) in /_/src/System.Runtime.Serialization.Formatters/tests/BinaryFormatterTests.cs:line 64

and

    System.Data.Tests.DataSetTest2.InferXmlSchema_inferringTables3 [FAIL]
      Assert.Equal() Failure
                � (pos 0)
      Expected: NewDataSet
      Actual:   DocumentElement
                � (pos 0)
      Stack Trace:
        /_/src/System.Data.Common/tests/System/Data/DataSetTest2.cs(755,0): at System.Data.Tests.DataSetTest2.InferXmlSchema_inferringTables3()

@eriawan
Copy link
Member Author

eriawan commented Oct 22, 2019

@stephentoub

Thanks for pointing the failed unit tests! Let me look at this again.

stephentoub
stephentoub approved these changes Oct 23, 2019
View reviewed changes
Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@stephentoub stephentoub merged commit ae71c86 into dotnet:master Oct 23, 2019
@eriawan eriawan deleted the remove_unused_locals_System.Data.Common branch October 25, 2019 17:48
@karelz karelz added this to the 5.0 milestone Dec 19, 2019
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
@eriawan @stephentoub
Remove unused locals in System.Data.Common and keep some logic using …
322d208 
…discard (dotnet/corefx#41962)

* remove unused locals in System.Data.Common and keep some logic using discard

* revert the HandleTable(element) removal as PR feedback. Use discard as return value is ignored.


Commit migrated from dotnet/corefx@ae71c86
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@stephentoub stephentoub stephentoub approved these changes

Assignees
No one assigned
Labels
area-System.Data
Projects
None yet
Milestone
5.0
Development

Successfully merging this pull request may close these issues.
4 participants
@eriawan @stephentoub @karelz @Dotnet-GitSync-Bot